frontend/src/app/components/dashboard/db-table-view/db-table-view.component.ts:163

• actionsColumnWidth doesn’t account for tables without primary keys. TablesDataSource omits the actions column when keyAttributes.length === 0, but this getter may still return a non-zero width (based on permissions/actions), which can put the table into “with actions” layout even though the column isn’t rendered. Consider returning '0' when !this.tableData?.keyAttributes?.length (or when displayedColumns doesn’t include actions).

	public displayedColumns: string[] = [];
	public columnsToDisplay: string[] = [];
	public searchString: string;
	public staticSearchString: string;
	public bulkActions: CustomAction[];
	public bulkRows: string[];
	public displayedComparators = {
		eq: '=',
		gt: '>',

frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.html (1)

25-25: Consider adding track to @for when migrating control flow.

The *ngFor loop iterating over savedFilterData lacks a trackBy function. When this file is migrated to built-in control flow syntax, ensure you add a track expression (e.g., track filter.id) for better performance.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.html`
at line 25, The *ngFor on the mat-chip-option iterating savedFilterData should
use a track expression to maintain stable identities during migration to control
flow; update the loop over savedFilterData (the mat-chip-option element using
*ngFor) to include a track clause such as tracking a unique property (e.g.,
track filter.id) or a trackBy function that returns a unique id for each filter
to improve rendering performance and avoid unnecessary re-renders.

frontend/src/app/components/users/user-add-dialog/user-add-dialog.component.html (1)

21-26: Consider using @else instead of a separate @if.

Since data.availableMembers.length and data.availableMembers.length === 0 are mutually exclusive, you can simplify this to an @else block:

♻️ Suggested refactor

     }
-    `@if` (data.availableMembers.length === 0) {
+    } `@else` {
       <p class="mat-body-2">
         All your company members are already in this group. To add a new one first add them to your company.
       </p>
-    }
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/users/user-add-dialog/user-add-dialog.component.html`
around lines 21 - 26, The second template conditional uses a separate "@if
(data.availableMembers.length === 0)" even though it is the exact inverse of the
preceding "@if (data.availableMembers.length > 0)"; change that second "@if" to
an "@else" so the mutually exclusive branches are simplified and easier to
maintain, locating the conditional that references data.availableMembers.length
in the user-add-dialog component template.

frontend/src/app/components/users/user-add-dialog/user-add-dialog.component.spec.ts (1)

55-65: Consider extracting testable type alias for consistency with other dialog tests.

Other dialog tests (e.g., UserDeleteDialogComponent, GroupDeleteDialogComponent) define a separate *Testable type alias at the top of the file. This inline type cast works but is inconsistent with the established pattern.

♻️ Suggested refactor for consistency

+type UserAddTestable = UserAddDialogComponent & {
+	groupUserEmail: string;
+	joinGroupUser: () => Promise<void>;
+};
+
 describe('UserAddDialogComponent', () => {
   // ...
   it('should call add user service', async () => {
-    const testable = component as unknown as UserAddDialogComponent & {
-      groupUserEmail: string;
-      joinGroupUser: () => Promise<void>;
-    };
+    const testable = component as unknown as UserAddTestable;
     testable.groupUserEmail = 'user@test.com';

Based on learnings: "Use testable type aliases to access protected signals in tests instead of as any".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/users/user-add-dialog/user-add-dialog.component.spec.ts`
around lines 55 - 65, The test uses an inline cast to access protected members
of UserAddDialogComponent; extract a reusable testable type alias (e.g.,
UserAddDialogComponentTestable) at the top of the spec to mirror other dialog
tests, then replace the inline cast with that alias when creating testable and
when accessing groupUserEmail and joinGroupUser so the spec is consistent with
UserDeleteDialogComponent and GroupDeleteDialogComponent patterns.

frontend/src/app/components/dashboard/db-tables-list/db-tables-list.component.ts (1)

123-128: Mixed dependency injection patterns: constructor injection and inject() in same component.

The component uses both constructor-based DI (lines 123-128) and inject() function (lines 80-81). Per coding guidelines, prefer the inject() function pattern consistently.

♻️ Suggested refactor for consistency

+	private _tableState = inject(TableStateService);
+	private _tablesService = inject(TablesService);
+	private _uiSettingsService = inject(UiSettingsService);
+	private dialog = inject(MatDialog);
+
-	constructor(
-		private _tableState: TableStateService,
-		private _tablesService: TablesService,
-		private _uiSettingsService: UiSettingsService,
-		private dialog: MatDialog,
-	) {}

Based on learnings: "Inject dependencies using the inject() function pattern".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/dashboard/db-tables-list/db-tables-list.component.ts`
around lines 123 - 128, The component mixes constructor-based DI with the
inject() function (TableStateService, TablesService, UiSettingsService,
MatDialog vs existing inject() calls) — refactor to use a single pattern by
replacing the constructor parameters (private _tableState: TableStateService,
private _tablesService: TablesService, private _uiSettingsService:
UiSettingsService, private dialog: MatDialog) with inject() calls at the top of
the class (matching the existing inject() style used earlier) so all
dependencies are obtained via inject() and remove the empty constructor; ensure
property names and usages (e.g., _tableState, _tablesService,
_uiSettingsService, dialog) remain the same.

frontend/src/app/components/connect-db/connect-db.component.ts (1)

123-139: Mixed DI patterns: constructor injection and inject() function.

The component uses constructor-based DI for most services but inject() for CedarPermissionService. Per coding guidelines, prefer the inject() function pattern consistently throughout.

Based on learnings: "Inject dependencies using the inject() function pattern (e.g., private _http = inject(HttpClient))".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/components/connect-db/connect-db.component.ts` around lines
123 - 139, Switch the component to use the inject() pattern for dependencies
instead of constructor injection: replace the constructor-based injections for
ConnectionsService (_connections), NotificationsService (_notifications),
UserService (_user), CompanyService (_company), NgZone (ngZone), Router
(router), MatDialog (dialog), Angulartics2 (angulartics2), and Title (title)
with class fields initialized via inject(); remove the constructor and any
direct uses that rely on it, and move the canEditConnection initialization
(which currently calls this._permissions.canI(...)) into a lifecycle hook or a
class field initializer that runs after injection (e.g., set canEditConnection
in ngOnInit using the injected services); ensure CedarPermissionService usage
remains via inject() and update references to those symbols accordingly.

frontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.spec.ts (1)

36-48: Consider extracting repeated type annotation.

The type { action: string; tableName?: string; dashboardId?: string }[] is repeated across multiple tests. Extracting it to a type alias would improve maintainability.

♻️ Suggested refactor

+type PolicyPayload = { action: string; tableName?: string; dashboardId?: string };
+
 describe('CedarPolicyListComponent', () => {
   // ...
   
   it('should add a policy', () => {
-    let emitted: { action: string; tableName?: string; dashboardId?: string }[] | null = null;
+    let emitted: PolicyPayload[] | null = null;
     component.policiesChange.subscribe((v) => (emitted = v));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.spec.ts`
around lines 36 - 48, The test repeats the inline type `{ action: string;
tableName?: string; dashboardId?: string }[]` for the emitted variable; extract
that to a local type alias (e.g., declare type PolicyList = { action: string;
tableName?: string; dashboardId?: string }[] ) at the top of the spec and
replace the inline annotations for `emitted` in the `it('should add a policy',
...)` test (and any other tests) so `emitted` is declared as `PolicyList |
null`, keeping the existing subscriptions to `component.policiesChange` and
assertions against `emitted` unchanged.

frontend/src/app/components/users/group-add-dialog/group-add-dialog.component.spec.ts (1)

62-71: Test doesn't verify dialog closes on success.

The test mocks mockDialogRef.close but doesn't assert it was called after successful group creation.

♻️ Add assertion for dialog close

 	it('should call create group service', async () => {
 		const testable = component as unknown as GroupAddTestable;
 		testable.connectionID = '12345678';
 		testable.groupTitle = 'Sellers';

 		await testable.addGroup();

 		expect(mockUsersService.createGroup).toHaveBeenCalledWith('12345678', 'Sellers');
 		expect(testable.submitting()).toBe(false);
+		expect(mockDialogRef.close).toHaveBeenCalled();
 	});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/users/group-add-dialog/group-add-dialog.component.spec.ts`
around lines 62 - 71, The test currently ensures mockUsersService.createGroup is
called and submitting() is false but does not assert the dialog is closed;
update the spec for GroupAddTestable.addGroup to also verify mockDialogRef.close
was invoked after a successful createGroup call (e.g., add an expectation that
mockDialogRef.close was called). Locate the test using symbols testable
(GroupAddTestable), addGroup(), mockUsersService.createGroup and
mockDialogRef.close and add the assertion immediately after the existing
expectations so the dialog-close behavior is validated.

frontend/src/app/components/db-table-row-edit/db-table-row-edit.component.ts (1)

124-125: Inconsistent dependency injection pattern.

CedarPermissionService uses inject() while other dependencies use constructor injection. Consider migrating all dependencies to the inject() pattern for consistency.

This is a larger refactor that could be addressed in a follow-up PR to maintain consistency across the codebase. As per coding guidelines: "Inject dependencies using the inject() function pattern."

Also applies to: 140-152

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/components/db-table-row-edit/db-table-row-edit.component.ts`
around lines 124 - 125, The file mixes constructor injection with the standalone
inject() API (CedarPermissionService is assigned to private _permissions via
inject() while other services are injected in the constructor); migrate the
remaining constructor-injected dependencies in db-table-row-edit.component (the
constructor parameters) to use the inject() pattern and replace constructor
usage accordingly so all services use inject() (e.g., change
constructor-injected services to private fields assigned via inject(), remove
the constructor or keep it only for non-injectable logic, and update any
references to those services within methods to use the new private fields) to
make dependency injection consistent with the
CedarPermissionService/_permissions approach.

frontend/src/app/services/users.service.spec.ts (1)

149-253: New signal-based tests provide good coverage.

The tests properly verify:

• Signal updates (groupsUpdated(), groupUsers())
• ApiService method calls with correct parameters
• Empty state handling ('empty' sentinel value)

Consider adding error scenario tests for the async methods to ensure proper error propagation.

♻️ Example error scenario test

it('should handle createGroup error', async () => {
  mockApi.post.mockRejectedValue(new Error('Network error'));

  await expect(service.createGroup('12345678', 'Managers')).rejects.toThrow('Network error');
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/services/users.service.spec.ts` around lines 149 - 253, Add
negative-path unit tests that assert async methods propagate errors: for
createGroup, deleteGroup, editGroupName, saveCedarPolicy, addGroupUser,
deleteGroupUser, and fetchGroupUsers, mock the corresponding
mockApi.post/put/delete/get to mockRejectedValue(new Error('Network error')) and
assert await expect(service.<method>(...)).rejects.toThrow('Network error'); for
fetchGroupUsers also assert that when get rejects the service does not set
groupUsers()['<id>'] to a success value (or explicitly stays undefined), and use
the existing test helpers (mockApi, service) to mirror the success tests' call
args and signal checks.

frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.ts (2)

100-106: Mixed dependency injection patterns.

The component uses both the inject() function pattern (lines 63-64) and constructor-based DI (lines 100-106). For consistency with the coding guidelines, consider migrating all dependencies to use inject().

As per coding guidelines: "Inject dependencies using the inject() function pattern"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.ts`
around lines 100 - 106, The constructor currently injects MatDialog,
TablesService, ConnectionsService, Router and ActivatedRoute while the file also
uses inject() elsewhere; replace these constructor-based injections by calling
inject() for each dependency (e.g., const dialog = inject(MatDialog), const
_tables = inject(TablesService), const _connections =
inject(ConnectionsService), const router = inject(Router), const route =
inject(ActivatedRoute)) and remove the constructor entirely; ensure the property
names match existing usages in SavedFiltersPanelComponent so references to
dialog, _tables, _connections, router and route continue to work.

---

63-69: canEditConnection is computed once at instantiation, not reactively.

The canEditConnection signal is computed once when the component is instantiated, using currentConnectionID at that moment. If the connection changes while the component is alive (unlikely but possible), this permission check won't update.

If the component is recreated on route changes, this is likely fine. However, consider using a computed() signal if reactive updates are needed:

protected canEditConnection = computed(() => 
  this._permissions.canI('connection:edit', 'Connection', this._connectionsForPermissions.currentConnectionID)()
);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.ts`
around lines 63 - 69, canEditConnection is evaluated once at instantiation and
won't update if the connection changes; wrap the permission check in a reactive
computed so it recalculates when currentConnectionID changes: replace the direct
assignment to protected canEditConnection with a computed(() => ...) that calls
this._permissions.canI('connection:edit', 'Connection',
this._connectionsForPermissions.currentConnectionID) and returns its current
boolean value (invoke the returned signal/function if canI returns one) so the
permission stays in sync with ConnectionsService.currentConnectionID.

frontend/src/app/components/dashboard/db-table-view/db-table-view.component.html (1)

106-109: Redundant aria-checked attributes.

The button has both a static aria-checked="false" and a dynamic [attr.aria-checked]="column.selected". The static attribute is redundant and will be overridden by the dynamic binding.

🔧 Proposed fix

 <button type="button" mat-menu-item
     role="menuitemcheckbox"
-    aria-checked="false"
     [attr.aria-checked]="column.selected"
     *ngFor="let column of sortedColumns"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/dashboard/db-table-view/db-table-view.component.html`
around lines 106 - 109, Remove the redundant static aria-checked="false" from
the menu button that already uses the dynamic binding
[attr.aria-checked]="column.selected" (the button with role="menuitemcheckbox");
keep the dynamic binding so the accessible checked state comes from
column.selected and delete the hard-coded attribute to avoid
conflicting/duplicated ARIA values.

frontend/src/app/components/users/users.component.spec.ts (1)

47-50: Signal mock casts use as any instead of testable type aliases.

The signal mocks are cast with as any which violates the coding guideline about using testable type aliases to access protected signals in tests. Consider defining proper type aliases or adjusting the mock structure to avoid as any.

-	groups: mockGroups.asReadonly() as any,
-	groupsLoading: signal(false).asReadonly() as any,
-	groupUsers: signal({}).asReadonly() as any,
-	groupsUpdated: signal('').asReadonly() as any,
+	groups: mockGroups.asReadonly() as UsersService['groups'],
+	groupsLoading: signal(false).asReadonly() as UsersService['groupsLoading'],
+	groupUsers: signal({}).asReadonly() as UsersService['groupUsers'],
+	groupsUpdated: signal('').asReadonly() as UsersService['groupsUpdated'],

Based on learnings: "Use testable type aliases to access protected signals in tests instead of as any"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/components/users/users.component.spec.ts` around lines 47 -
50, Replace the unsafe "as any" casts for the mocked signals (groups,
groupsLoading, groupUsers, groupsUpdated) with explicit testable type aliases
that match the component's protected signal types; define a local alias (e.g.,
TestSignal<T> matching the Signal/WritableSignal shape used by the component)
and cast each mock to that alias instead of any so tests can read/inspect the
signal values while preserving type safety for groups, groupsLoading, groupUsers
and groupsUpdated.

frontend/src/app/components/dashboard/db-table-view/db-table-view.component.ts (1)

155-163: Consider converting actionsColumnWidth to a computed() signal.

The actionsColumnWidth getter recalculates on every access. Since it depends on signal values (canEditRow(), canAddRow(), canDeleteRow()), converting it to a computed() signal would provide memoization benefits.

♻️ Suggested refactor

-protected get actionsColumnWidth(): string {
-  const editCount = this.canEditRow() ? 1 : 0;
-  const addCount = this.canAddRow() ? 1 : 0;
-  const deleteCount = this.canDeleteRow() && this.tableData?.canDelete ? 1 : 0;
-  const defaultActionsCount = editCount + addCount + deleteCount;
-  const totalActionsCount = (this.tableData?.tableActions?.length || 0) + defaultActionsCount;
-  const lengthValue = totalActionsCount * 30 + 32;
-  return totalActionsCount === 0 ? '0' : `${lengthValue}px`;
-}
+protected actionsColumnWidth = computed(() => {
+  const editCount = this.canEditRow() ? 1 : 0;
+  const addCount = this.canAddRow() ? 1 : 0;
+  const deleteCount = this.canDeleteRow() && this.tableData?.canDelete ? 1 : 0;
+  const defaultActionsCount = editCount + addCount + deleteCount;
+  const totalActionsCount = (this.tableData?.tableActions?.length || 0) + defaultActionsCount;
+  const lengthValue = totalActionsCount * 30 + 32;
+  return totalActionsCount === 0 ? '0' : `${lengthValue}px`;
+});

Note: This would require updating template usages from actionsColumnWidth to actionsColumnWidth().

As per coding guidelines: "Use computed() for derived state in components and services"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/dashboard/db-table-view/db-table-view.component.ts`
around lines 155 - 163, The getter actionsColumnWidth currently recomputes on
every access; replace it with a memoized computed signal by creating
actionsColumnWidth = computed(() => { ...same logic... }) that reads
canEditRow(), canAddRow(), canDeleteRow(), and this.tableData signals inside the
computed callback; update any template or internal usages from property access
to a function call (actionsColumnWidth()) and remove the existing protected get
actionsColumnWidth() getter so only the computed signal (imported from
`@angular/core`) remains.

frontend/src/app/components/users/group-name-edit-dialog/group-name-edit-dialog.component.ts (1)

27-31: Rename addGroup() to match the edit flow.

This method calls editGroupName(), so addGroup() is misleading in the template and future call sites. saveGroupName() or renameGroup() would describe the current behavior.

As per coding guidelines, frontend/**/*.ts: Use camelCase for method names with verb-noun pattern.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/users/group-name-edit-dialog/group-name-edit-dialog.component.ts`
around lines 27 - 31, The method name addGroup() is misleading because it calls
editGroupName(); rename addGroup() to a camelCase verb-noun name such as
saveGroupName() or renameGroup() (e.g., saveGroupName) and update all references
(component template bindings and any callers) to the new method name so the edit
flow is clear; ensure the method body (including
this._usersService.editGroupName and this.dialogRef.close) remains unchanged and
update any tests or event handlers that referenced addGroup().

frontend/src/app/components/users/user-add-dialog/user-add-dialog.component.ts (1)

26-32: Type the dialog payload instead of using any.

This flow depends on data.group.id, but any removes the compile-time check and the file still mixes constructor injection for MAT_DIALOG_DATA into otherwise inject()-based code. A small UserAddDialogData interface plus inject(MAT_DIALOG_DATA) would catch bad callers early.

As per coding guidelines, frontend/**/*.ts: Inject dependencies using the inject() function pattern (e.g., private _http = inject(HttpClient)) and **/*.{ts,tsx}: Avoid any types - use specific types or generics.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/users/user-add-dialog/user-add-dialog.component.ts`
around lines 26 - 32, Replace the any-typed constructor injection with a typed
dialog payload and use Angular's inject() helper: define an interface
UserAddDialogData (e.g., { group: { id: string } } with the correct id type),
remove the constructor(`@Inject`(MAT_DIALOG_DATA) public data: any) pattern, and
instead create a class field like private data =
inject<UserAddDialogData>(MAT_DIALOG_DATA); update usages in joinGroupUser and
elsewhere (this.data.group.id, this.groupUserEmail, _usersService.addGroupUser)
to rely on that typed field and drop the biome-ignore comment so callers are
type-checked.

frontend/src/app/services/users.service.ts (1)

141-190: Legacy Observable methods retained for compatibility.

Keeping fetchConnectionUsers, fetchPermission, and updatePermission as Observable-based methods is pragmatic for incremental migration. Consider adding a comment or TODO indicating these could be migrated in a future PR.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/services/users.service.ts` around lines 141 - 190, Add a
short comment/TODO above the legacy Observable-style methods
fetchConnectionUsers, fetchPermission, and updatePermission indicating they are
kept for compatibility with AuditComponent/permissions UI and could be migrated
to Promise/async-await (HTTPClient.toPromise()/firstValueFrom) in a future PR;
place the note near the top of the block (above fetchConnectionUsers) and
mention the specific callers (AuditComponent / permissions UI) so future
refactors know why they remain.

frontend/src/app/components/users/users.component.ts (1)

78-93: Effect-based event handling pattern is correct but may benefit from documentation.

The pattern of reading groupsUpdated(), processing based on the event type, then calling clearGroupsUpdated() within the same effect is valid. The clear at the end ensures the effect doesn't re-trigger for the same event. However, this pattern might be non-obvious to future maintainers.

Consider adding a brief comment explaining the event-processing-and-clear pattern.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/components/users/users.component.ts` around lines 78 - 93,
Add a short explanatory comment above the effect describing the
read-process-clear event pattern: that effect() reads
this._usersService.groupsUpdated() once, branches on the returned action
('user-added'|'user-deleted' vs others), triggers either
fetchAllGroupUsers(currentGroups) or refreshGroups(), and then calls
clearGroupsUpdated() to prevent retriggering for the same event; reference the
involved symbols effect, this._usersService.groupsUpdated(),
this._usersService.clearGroupsUpdated(), fetchAllGroupUsers, refreshGroups, and
groups so future maintainers understand why the clear is inside the same effect.

frontend/src/app/lib/cedar-policy-roundtrip.spec.ts (1)

52-56: Consider adding error handling to WASM initialization in beforeAll.

If the WASM fetch fails (e.g., asset path misconfigured in test environment), all tests will fail with unclear errors. Adding explicit error handling would improve debuggability.

✨ Proposed improvement

 beforeAll(async () => {
-	const wasmBytes = await fetch('/assets/cedar-wasm/cedar_wasm_bg.wasm').then((r) => r.arrayBuffer());
-	initSync({ module: new WebAssembly.Module(wasmBytes) });
+	const response = await fetch('/assets/cedar-wasm/cedar_wasm_bg.wasm');
+	if (!response.ok) {
+		throw new Error(`Failed to load Cedar WASM: ${response.status} ${response.statusText}`);
+	}
+	const wasmBytes = await response.arrayBuffer();
+	initSync({ module: new WebAssembly.Module(wasmBytes) });
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/lib/cedar-policy-roundtrip.spec.ts` around lines 52 - 56,
The WASM initialization in the beforeAll block (the fetch call for
'/assets/cedar-wasm/cedar_wasm_bg.wasm' and the initSync({ module: new
WebAssembly.Module(wasmBytes) })) lacks error handling; wrap the fetch +
WebAssembly.Module creation and initSync call in a try/catch, and on error throw
or fail the test with a clear message including the failing URL and the caught
error (or call console.error then rethrow) so test failures show a descriptive
initialization error instead of an opaque failure.

frontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.ts (1)

204-206: Consider using computed() for editActionGroups.

editActionGroups is a getter that calls _buildFilteredGroups() on every access. If this is accessed multiple times in the template, it will recompute each time. Converting it to a computed() based on editingIndex would cache the result.

♻️ Proposed refactor to use computed()

+	private _editingIndex = signal<number | null>(null);
+
+	get editingIndex(): number | null {
+		return this._editingIndex();
+	}
+	set editingIndex(value: number | null) {
+		this._editingIndex.set(value);
+	}
+
+	protected editActionGroups = computed(() => this._buildFilteredGroups(this._editingIndex() ?? -1));

-	get editActionGroups(): PolicyActionGroup[] {
-		return this._buildFilteredGroups(this.editingIndex ?? -1);
-	}

Alternatively, if the template only accesses this once, the current implementation is acceptable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.ts`
around lines 204 - 206, The getter editActionGroups currently calls
_buildFilteredGroups(this.editingIndex ?? -1) on every access which can cause
repeated recomputation; replace the getter with a Vue computed property (import
computed from 'vue') that returns computed(() =>
this._buildFilteredGroups(this.editingIndex ?? -1)) so the result is cached and
only recalculated when editingIndex changes, and update references to use the
computed property consistently in the component/template.

frontend/src/app/services/cedar-permission.service.ts (2)

35-51: Potential unbounded cache growth in _signals Map.

The memoization Map grows without bound as new (action, resourceType, resourceId) combinations are requested. In long-running sessions with many dynamic resource IDs, this could lead to memory pressure.

Consider using a WeakRef-based cache, an LRU eviction strategy, or clearing the cache when the user changes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/services/cedar-permission.service.ts` around lines 35 - 51,
The _signals Map used by canI(action, resourceType, resourceId) can grow
unbounded; modify the caching strategy to prevent memory leaks by replacing or
augmenting _signals with a bounded cache (e.g., an LRU with a max size) or use
WeakRef/FinalizationRegistry for entries tied to object keys, and ensure you
evict entries when the user context changes; update canI to insert into that
bounded cache and ensure computed signals created via computed(() =>
this._evaluate(...)) are removed/cleaned on eviction or user change, and add a
clearAll/clearForUser method (or integrate eviction in the
constructor/user-change handler) so _signals no longer grows indefinitely.

---

73-78: Silent permission denial on Cedar errors may obscure policy issues.

When isAuthorized returns an error, the code logs a warning but returns false, which denies access. This could make it difficult to diagnose malformed policies in production since users would simply see access denied without understanding why.

Consider exposing error state or distinguishing between "explicitly denied" and "evaluation failed" in the return type.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/services/cedar-permission.service.ts` around lines 73 - 78,
The current isAuthorized implementation masks Cedar evaluation failures by
logging and returning false; update isAuthorized in cedar-permission.service.ts
to return a richer result (e.g., a union/DTO like { allowed: boolean; error?:
any; status: 'allowed'|'denied'|'failed' }) instead of a plain boolean so
callers can distinguish explicit denials from evaluation failures, and on
non-success (when result.type !== 'success') populate the error/details and
status='failed' (or alternatively throw a descriptive error) while keeping the
existing success path mapping decision === 'allow' to allowed=true and
status='allowed'.

frontend/src/app/components/dashboard/db-tables-data-source.ts (1)

340-349: Remove unused method parameters or document API consistency requirement.

The method changleColumnList receives _connectionId and _tableName parameters but does not use them. The underscore prefix indicates intentional disuse. Since the method is called from db-table-view.component.ts:727 with both parameters, either remove them and update the caller, or document if these parameters are required for API consistency.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/components/dashboard/db-tables-data-source.ts` around lines
340 - 349, The method changleColumnList currently declares unused parameters
(_connectionId, _tableName); either remove those parameters from
changleColumnList and update its caller in db-table-view.component.ts (the call
at or near line where changleColumnList is invoked) to call it without
arguments, or if API consistency requires the parameters, keep them but remove
the underscore and add a short JSDoc on changleColumnList explaining they are
intentionally accepted for signature compatibility (and reference
connectionId/tableName) so they are not reported as unused; update all call
sites accordingly to match the chosen approach.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

frontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.ts (1)

203-205: Consider relocating getter to maintain component structure.

The editActionGroups getter is placed between public methods (startEdit and saveEdit). As per component structure guidelines, derived state (getters/computed) should be placed before public methods.

Consider moving this getter up near lines 71-73 where other derived state is declared.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.ts`
around lines 203 - 205, The editActionGroups getter (which calls
_buildFilteredGroups(this.editingIndex ?? -1)) is currently located between
public methods startEdit and saveEdit; move this derived-state getter up into
the component's derived/computed section (near where other getters around lines
with existing derived state are declared, e.g. alongside other getters at
~71-73) so that all computed/derived properties appear before public methods;
ensure references to editingIndex and _buildFilteredGroups remain unchanged.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

frontend/src/app/components/users/users.component.ts (2)

105-107: ⚠️ Potential issue | 🟠 Major

Make connection-scoped state follow the current connection instead of one-time reads.

This component stores currentConnectionID locally, but never seeds UsersService.setActiveConnection() from it, and canCreateGroup captures _connections.currentConnectionID once at field initialization. That leaves group loading and create-permission checks dependent on whichever connection id happened to exist before ngOnInit().

Also applies to: 126-126

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/components/users/users.component.ts` around lines 105 - 107,
The component currently snapshots _connections.currentConnectionID once (at
field init and in ngOnInit) which breaks connection-scoped state; change it to
react to connection changes by calling UsersService.setActiveConnection whenever
the connection ID changes and by computing canCreateGroup from the live
connection (e.g., subscribe to _connections.connectionId$ or
_connections.onChange and inside that subscription set this.connectionID and
call this.usersService.setActiveConnection(connectionID)), remove any one-time
capture of _connections.currentConnectionID (including the canCreateGroup field
initialization) and replace with either a getter or observable-driven value so
group loading and permission checks always use the current connection ID.

---

117-123: ⚠️ Potential issue | 🟡 Minor

The inner company-members subscription is still unbounded.

The outer cast subscription is torn down, but each inner fetchCompanyMembers() subscription lives independently. If cast emits again before an earlier request completes, those requests can overlap and race to update companyMembers.

🛠️ Suggested change

-import { take } from 'rxjs';
+import { switchMap, take, tap } from 'rxjs';
@@
-		this._userService.cast.pipe(takeUntilDestroyed(this._destroyRef)).subscribe((user) => {
-			this.currentUser.set(user);
-
-			this._company.fetchCompanyMembers(user.company.id).subscribe((members) => {
-				this.companyMembers.set(members);
-			});
-		});
+		this._userService.cast
+			.pipe(
+				takeUntilDestroyed(this._destroyRef),
+				tap((user) => {
+					this.currentUser.set(user);
+				}),
+				switchMap((user) => this._company.fetchCompanyMembers(user.company.id)),
+			)
+			.subscribe((members) => {
+				this.companyMembers.set(members);
+			});

Based on learnings: Applies to frontend/**/*.component.ts : Use takeUntil pattern for memory leak prevention with proper subscription management.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/components/users/users.component.ts` around lines 117 - 123,
The inner subscription to _company.fetchCompanyMembers is unbounded and can
overlap; refactor the _userService.cast pipeline to avoid nested subscribes by
using an operator like switchMap: pipe takeUntilDestroyed(this._destroyRef), tap
to call currentUser.set(user), then switchMap(user =>
_company.fetchCompanyMembers(user.company.id)) so the previous request is
cancelled if a new user emits, and finally subscribe once to set companyMembers
(companyMembers.set). Ensure the takeUntilDestroyed(this._destroyRef) remains on
the outer stream so teardown works.

frontend/src/app/services/users.service.ts (1)

90-106: ⚠️ Potential issue | 🟠 Major

Only emit groupsUpdated after a successful mutation.

deleteGroup, editGroupName, addGroupUser, and deleteGroupUser still fire update events unconditionally. createGroup() already shows the safer pattern in this file: gate the event behind a successful response so failed writes do not trigger stale refreshes or success-looking UI flows.

Also applies to: 120-139

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/services/users.service.ts` around lines 90 - 106, The
deleteGroup, editGroupName, addGroupUser, and deleteGroupUser functions
currently call this._groupsUpdated unconditionally; change them to only emit
after confirming the API call succeeded (matching the safer pattern used in
createGroup). Specifically, capture the result of this._api.delete /
this._api.put / this._api.post calls, check the success/ok flag returned by the
API response (or that no error was thrown), and only call
this._groupsUpdated.set('...') when the response indicates a successful
mutation; update deleteGroup, editGroupName, addGroupUser, and deleteGroupUser
accordingly.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes